generate random seeds for IndexDistribution upon creation of income d… #1380
Conversation
…istribution, fixes #i1379
|
@alanlujan91 Could I please ask for an expedited review/merge of this PR, as it is a blocker for deadline-sensitive work? |
|
The errors this PR is getting are like this: Which has to do with numba dependencies which have nothing to do with this PR. |
There was a problem hiding this comment.
Having trouble testing this locally because of numba installation errors.
The code changes look good to me, everything should be passed a seed and an RNG. But shouldn't the RNG be initialized with that seed?
seed = self.RNG.integers(0, 2**31 - 1)
RNG = self.RNG(seed)
or something to that effect?
Anyway if this is generating better p_shk streams then it's a good fix for now.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1380 +/- ##
=======================================
Coverage 71.58% 71.59%
=======================================
Files 84 84
Lines 13929 13933 +4
=======================================
+ Hits 9971 9975 +4
Misses 3958 3958 ☔ View full report in Codecov by Sentry. |
|
I'm honestly quite confused by the RNG argument. But the problem in this case was much dumber: it was that even with a seed set for an AgentType, a default seed (0) was being used for income distributions. This is much easier to fix. |
|
Seems like there should be a way to make the seed globally available. Probably would be a good idea to tack on the seed to the filename (or record it in some other way) for any output files created. And, if it's a date seed, that would make it possible to explore the history of versions of the output files by date. |
|
The issue here was simply that the intended RNG structure wasn't being
used. Each instance of AgentType has a single seed value, which is used for
its RNG attribute (a numpy.random.RandomState instance). That RNG is
supposed to be used to generate seeds for all of the distributions *within*
an AgentType instance.
In applications/projects with only one type, the user can set that top
level seed manually. For projects with multiple types, they should have a
higher level seed generator, and set the seed for *that* manually. The
principle is that for any given project, there is usually *one*
human-controlled seed.
In this case, the AgentType-level seed generator was not setting seeds for
(at least) one kind of distribution, so no matter what seed you set at the
top level, income shock draws would come out the same. The only reason they
were diverging in later periods (I think) is different mortality draws,
which *were* using the intended structure.
…On Wed, Feb 7, 2024, 9:25 AM Christopher Llorracc Carroll < ***@***.***> wrote:
Seems like there should be a way to make the seed globally available.
Probably would be a good idea to tack on the seed to the filename (or
record it in some other way) for any output files created. And, if it's a
date seed, that would make it possible to explore the history of versions
of the output files by date.
—
Reply to this email directly, view it on GitHub
<#1380 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADKRAFJ2NGIBWBGJ2JJEP73YSOFFVAVCNFSM6AAAAABC52EGGOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZSGE2TQNZWHE>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
fixes #1379 by generating random seeds from the AgentType's RNG when initializing the IndexDistributions for PermSfkDst and other distributions in
construct_lognormal_income_process_unemploymentPlease ensure your pull request adheres to the following guidelines: